Skip to content

Conversation

@blva
Copy link
Collaborator

@blva blva commented Feb 18, 2025

Proposed changes

Jira ticket: CLOUDP-297223

  • Generates private and public specs that contain only the target endpoints

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works

Changes to Spectral

  • I have read the README file for Spectral Updates

Further comments

}

// WithFullContent returns an Option to generate a new APIVersion given the contentType and contentValue.
func WithFullContent(contentType string, contentValue *openapi3.MediaType) Option {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

core change: we need to consider contentValue to gather version data, since for preview and private preview we have the same content type atlas.vnd.preview+json, we can only differentiate them based on the x-xgen-preview extension

@blva blva marked this pull request as ready for review February 19, 2025 14:40
@blva blva requested a review from a team as a code owner February 19, 2025 14:40
Copy link
Member

@jeroenvervaeke jeroenvervaeke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one suggestion

}

func (v *APIVersion) IsPrivatePreview() bool {
return strings.Contains(v.version, PrivatePreviewStabilityLevel)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you call IsPreviewSabilityLevel(v.version) here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsPreviewSabilityLevel returns true to either Public or Private previews so we need to check for private preview here, but open to other suggestions

@andreaangiolillo andreaangiolillo changed the title CLOUDP-297223: Add support for private and public preview spec genera… CLOUDP-297223: Add support for private and public preview spec generation Feb 19, 2025
Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great work

}
}

func TestSplitPublicPreviewRun(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use the command locally to generate the spec and I noticed that the private preview file has several components that I think are not needed. Is this expected?
test-private-preview-info-resource.json

I am wondering if this is okay and will be fixed once we add the filter to remove the unused view 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should cover removing unused stuff after split together, for now we're validating we're filtering out the paths, which is the most important resource

}

// GetPreviewVersionName returns the preview version name.
func GetPreviewVersionName(contentTypeValue *openapi3.MediaType) (name string, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be public? 👀

Suggested change
func GetPreviewVersionName(contentTypeValue *openapi3.MediaType) (name string, err error) {
func getPreviewVersionName(contentTypeValue *openapi3.MediaType) (name string, err error) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes cause we use it in the versions package

@blva blva merged commit 0a00ee0 into main Feb 19, 2025
11 checks passed
@blva blva deleted the CLOUDP-297223 branch February 19, 2025 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants